Fast fail when reading failed in ComposedClientReadHandler#213
Fast fail when reading failed in ComposedClientReadHandler#213roryqi merged 1 commit intoapache:masterfrom
Conversation
|
Ping @frankliee Could help review this PR? If OK, I think I will go ahead. |
|
Like discussion at dev mail list, we will freeze the code and cut 0.6 version branch in September 15, we will not merge this pr before I cut 0.6 version branch, are you ok? |
integration-test/common/src/test/java/org/apache/uniffle/test/QuorumTest.java
Outdated
Show resolved
Hide resolved
zuston
force-pushed
the
inconsistentRead
branch
from
1dc685e to
3f78ca3
Compare
Codecov Report
@@ Coverage Diff @@
## master #213 +/- ##
============================================
- Coverage 59.10% 59.07% -0.03%
- Complexity 1326 1327 +1
============================================
Files 160 161 +1
Lines 8727 8736 +9
Branches 817 817
============================================
+ Hits 5158 5161 +3
- Misses 3303 3310 +7
+ Partials 266 265 -1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| if (!baseFolder.exists()) { | ||
| // the partition doesn't exist in this base folder, skip | ||
| throw new RuntimeException("Can't find folder " + fullShufflePath); | ||
| throw new FileNotFoundException("Can't find folder " + fullShufflePath); |
There was a problem hiding this comment.
Change exception to FileNotFoundException ，this should not be thrown to client.
For example:
When using the MEMORY_LOCALFILE_HDFS type, sometime, the localfile storage may be empty due to the big event flushed to HDFS directly. So when I want to fast fail in ComposedClientReadHandler, this problem should be involved. So this exception should be handled in server's grpc api and return the empty index result.
|
Ping @frankliee |
|
PTAL @frankliee |
|
|
||
| package org.apache.uniffle.common.exception; | ||
|
|
||
| public class FileNotFoundException extends RuntimeException { |
There was a problem hiding this comment.
Why not use Java.io.FileNotFoundException?
There was a problem hiding this comment.
Java.io.FileNotFoundException is IOException which need to catch in invoking side, so introduce this extending the RuntimeException.
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java
Outdated
Show resolved
Hide resolved
zuston
force-pushed
the
inconsistentRead
branch
from
09bb8f6 to
a323805
Compare
storage/src/main/java/org/apache/uniffle/storage/handler/impl/ComposedClientReadHandler.java
Show resolved
Hide resolved
roryqi
left a comment
roryqi
left a comment
There was a problem hiding this comment.
LGTM, thanks @zuston @frankliee
4 participants
What changes were proposed in this pull request?
Fast fail when reading failed in ComposedClientReadHandler
Why are the changes needed?
It should fast fail on network failure when reading shuffle data from memory/localfile/hdfs in
ComposedClientReadHandler. If not, it will throw inconsistent blockIds and make users confused.Does this PR introduce any user-facing change?
No
How was this patch tested?
UTs.